Skip to content

fix: EXPOSED-1048 Return only the rows batchInsert actually inserted - #2877

Open
obabichevjb wants to merge 1 commit into
mainfrom
obabichev/exposed-1048-batch-insert-returns
Open

fix: EXPOSED-1048 Return only the rows batchInsert actually inserted#2877
obabichevjb wants to merge 1 commit into
mainfrom
obabichev/exposed-1048-batch-insert-returns

Conversation

@obabichevjb

Copy link
Copy Markdown
Collaborator

Description

Summary of the change: batchInsert(ignore = true) no longer returns rows the database skipped on conflict.

Detailed description:

  • Why: batchInsert documents its return as "a list of ResultRow representing data from each newly inserted row", but built one row per submitted argument set regardless of what the database did with it. When INSERT IGNORE / ON CONFLICT DO NOTHING skipped a row, the caller still got it back as if it had been inserted, with no way to tell the difference — batchInsert returns a List<ResultRow> and discards the statement, so insertedCount is not reachable.

  • What:

    • InsertBlockingExecutable (JDBC) and InsertSuspendExecutable (R2DBC) now drop the argument sets the database reported no rows for, before building the result rows.
  • How: The per-entry affected-row counts already existed in both drivers and were being discarded.

    • JDBCexecuteBatch() returns List<Int>, already normalised by JdbcPreparedStatementImpl (SUCCESS_NO_INFO → 1, EXECUTE_FAILED → 0). It was being .sum()ed away in execInsertFunction; it is now captured first.
    • R2DBCresultSetsCounts was collected inside returnedValues() and dropped at the return statement. That function now returns a small private ReturnedValues holder instead of a Pair, and is called from executeInternal so the affected count is known before the rows are built.

    Both drivers then run the same filter:

    private fun List<List<Pair<Column<*>, Any?>>>.insertedOnly(
        inserted: Int,
        perArgumentSet: List<Int>?
    ): List<List<Pair<Column<*>, Any?>>> {
        if (statement !is BatchInsertStatement || !statement.isIgnore) return this
        if (inserted == 0) return emptyList()
        val counts = perArgumentSet?.takeIf { it.size == size } ?: return this
        return filterIndexed { index, _ -> counts[index] != 0 }
    }

    Two deliberate limits on the scope:

    • The filter only applies to a BatchInsertStatement with isIgnore. A single insertIgnore still reports its submitted values and leaves insertedCount to say whether the row was inserted — behaviour that testInsertIgnoreAndGetIdWithPredefinedId locks in. The BatchInsertStatement check also excludes BatchUpsertStatement and BatchReplaceStatement, which extend it with ignore = false; filtering those would drop rows from an upsert that MySQL reports as affecting 0 rows.
    • No public API changed. execInsertFunction is protected open and is in the API dump, so widening its return type to carry the counts would have been a binary-incompatible change. The JDBC counts travel in a private field instead, which also keeps them reachable when the ResultSet is null — the case for a UUIDTable with no generated columns on H2, MariaDB, SQL Server and Oracle, which is exactly the table shape in the report.

    Known limitation: telling apart which rows of a partly inserted batch were skipped needs an update count per statement, and the H2 and MariaDB R2DBC drivers send none. On those two, such a batch still returns all of its arguments, as before.


Type of Change

Please mark the relevant options with an "X":

  • Bug fix

Affected databases:

  • MariaDB
  • Mysql5
  • Mysql8
  • Oracle
  • Postgres
  • SqlServer
  • H2
  • SQLite

Oracle and SQL Server are unaffected: neither supports INSERT IGNORE, and the change is gated behind isIgnore.


Related Issues

EXPOSED-1048 https://youtrack.jetbrains.com/issue/EXPOSED-1048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant